Medicine administration UI #10289
Medicine administration UI #10289
Conversation
…medicine-administration-ui
…medicine-administration-ui
…medicine-administration-ui
…medicine-administration-ui
…medicine-administration-ui
…medicine-administration-ui
…medicine-administration-ui
…medicine-administration-ui
…r Questionnaire prefill
…medicine-administration-ui
WalkthroughThis pull request introduces comprehensive enhancements to medication administration functionality across multiple components. The changes include new components for managing medication administration, updates to localization files, modifications to existing patient and encounter-related components, and the addition of new API endpoints and type definitions. The implementation focuses on providing a more robust and user-friendly interface for healthcare professionals to track, administer, and manage medication requests. Changes
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (3)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (2)
Line range hint
291-297: Add encounterId to the query key for proper cache invalidation.The query won't automatically refetch when the encounter changes because
encounterIdis not included in the query key. This could lead to stale data being displayed.Apply this diff to ensure proper cache invalidation:
const { data: patientSymptoms } = useQuery({ - queryKey: ["symptoms", patientId], + queryKey: ["symptoms", patientId, encounterId], queryFn: query(symptomApi.listSymptoms, { pathParams: { patientId }, queryParams: { encounter: encounterId, limit: 100, }, }), });
Line range hint
300-311: Add missing dependencies to useEffect.The effect's dependency array is missing
updateQuestionnaireResponseCBandquestionnaireResponse.question_id. This could lead to stale closures and potentially cause updates to be lost.Apply this diff to fix the dependency array:
useEffect(() => { if (patientSymptoms?.results) { updateQuestionnaireResponseCB( [ { type: "symptom", value: patientSymptoms.results.map(convertToSymptomRequest), }, ], questionnaireResponse.question_id, ); } - }, [patientSymptoms]); + }, [patientSymptoms, updateQuestionnaireResponseCB, questionnaireResponse.question_id]);src/components/Patient/PatientDetailsTab/index.tsx (1)
Remove redundant
patientIdpropThe
patientIdprop is redundant with the existingidprop. The codebase consistently usesidas the primary identifier, and whenpatientIdis needed, it's derived from eitheridorpatientData.id. To maintain consistency and avoid confusion, use only theidprop.🔗 Analysis chain
Line range hint
13-17: Verify patientId prop usage.The
patientIdprop has been added toPatientProps, but it seems redundant with the existingidprop.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if patientId and id are used differently rg -A 5 "patientId.*=.*props" src/ rg -A 5 "id.*=.*props" src/Length of output: 17668
🧹 Nitpick comments (14)
src/components/ui/button.tsx (1)
45-47: Consider documenting the default variant change.Since this is a breaking change that could affect the appearance of buttons across the application, consider:
- Adding a comment explaining why "primary" is now the default variant
- Updating component documentation if it exists
- Communicating this change to other developers
src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (1)
87-87: Consider adding error handling for invalid encounter IDs.While the implementation is correct, consider adding validation or error handling for cases where the
encounterIdmight be invalid or empty.export function DiagnosisQuestion({ encounterId, patientId, questionnaireResponse, updateQuestionnaireResponseCB, disabled, }: DiagnosisQuestionProps) { + if (!encounterId?.trim()) { + console.warn('Invalid or empty encounter ID provided'); + } const diagnoses = (questionnaireResponse.values?.[0]?.value as DiagnosisRequest[]) || []; const { data: patientDiagnoses } = useQuery({Also applies to: 101-101
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
81-81: LGTM! Consider documenting the encounter-specific behavior.The implementation correctly adds encounter context to medication requests. Consider adding a comment explaining how encounter filtering affects medication requests visibility.
interface MedicationRequestQuestionProps { patientId: string; questionnaireResponse: QuestionnaireResponse; updateQuestionnaireResponseCB: ( values: ResponseValue[], questionId: string, note?: string, ) => void; disabled?: boolean; + /** + * Encounter ID used to filter medication requests. + * When provided, only shows medications associated with this encounter. + */ encounterId: string; }Also applies to: 89-89, 99-99
src/components/Medicine/MedicationAdministration/AdministrationTab.tsx (3)
43-48: Ensure extensibility for alternate time partitions
The hard-coded time slots could be made configurable if the application needs to adapt to different shift schedules or time block conventions in the future. Storing them in a separate configuration (or retrieving from an endpoint) might allow more flexibility.
65-85: Consider extracting visible slot calculation logic
The memoized calculation of visible time slots is rather involved and could be extracted into a custom hook or separate utility. This approach will improve readability and testability.
549-557: Validate safety checks before administering
The “Administer” button is only shown whenmedication.status === "active"in the current time slot. Ensure that all necessary distribution, ingestion, or cross-verification checks are performed prior to pediatric or high-risk medication administration.src/components/Medicine/MedicationAdministration/utils.ts (1)
26-33: Simplify the dose property assignment.The code has repetitive access to the dose quantity object, which can be simplified.
Apply this diff to reduce repetition:
- dose: medication.dosage_instruction[0]?.dose_and_rate?.dose_quantity && { - value: - medication.dosage_instruction[0]?.dose_and_rate?.dose_quantity?.value, - unit: medication.dosage_instruction[0]?.dose_and_rate?.dose_quantity - ?.unit, - code: medication.dosage_instruction[0]?.dose_and_rate?.dose_quantity - ?.unit, + dose: (() => { + const doseQuantity = medication.dosage_instruction?.[0]?.dose_and_rate?.dose_quantity; + return doseQuantity && { + value: doseQuantity.value, + unit: doseQuantity.unit, + code: doseQuantity.unit, + }; + })(),src/types/emr/medicationAdministration/medicationAdministration.ts (1)
56-96: Consider extracting common properties into a shared interface.The
MedicationAdministrationRequestandMedicationAdministrationReadinterfaces share many common properties. Consider creating a base interface to reduce duplication:interface BaseMedicationAdministration { status: MedicationAdministrationStatus; status_reason?: Code; medication: Code; occurrence_period_start: string; occurrence_period_end?: string; recorded?: string; note?: string; dosage?: { text?: string; site?: Code; route?: Code; method?: Code; dose?: DosageQuantity; rate?: Quantity; }; } export interface MedicationAdministrationRequest extends BaseMedicationAdministration { id?: string; encounter: string; request: string; } export interface MedicationAdministrationRead extends BaseMedicationAdministration { id: string; encounter: string; request: string; }src/components/Medicine/MedicationRequestTable/MedicineItem.tsx (1)
23-74: Enhance accessibility with ARIA labels and better contrast.Consider the following improvements:
- Add
aria-labelto the grid container- Use semantic HTML elements for the medication name (
h3is good)- Ensure text contrast meets WCAG guidelines, especially for
text-muted-foreground- <div className="space-y-1"> + <div className="space-y-1" role="article" aria-label={`Medication details for ${medication.medication?.display}`}>src/components/Medicine/MedicationAdministration/MedicineAdminDialog.tsx (1)
43-46: Prevent potential race conditions in useEffect.The current useEffect implementation might cause race conditions if initialRequest changes rapidly. Consider adding a cleanup function:
React.useEffect(() => { + let isActive = true; - setAdministrationRequest(initialRequest); + if (isActive) { + setAdministrationRequest(initialRequest); + } + return () => { + isActive = false; + }; }, [initialRequest]);src/components/Medicine/MedicationAdministration/MedicineAdminForm.tsx (1)
181-195: Consider consolidating date/time picker logic.The start and end time picker implementations share similar logic. Consider extracting this into a reusable component to reduce code duplication.
Also applies to: 197-220
src/components/Medicine/MedicationAdministration/MedicineAdminSheet.tsx (1)
242-248: Consider adding loading state to submit button.The submit button should show a loading state during mutation to prevent double submissions.
<Button type="submit" className="bg-[#006D4C] hover:bg-[#006D4C]/90" - disabled={selectedMedicines.size === 0} + disabled={selectedMedicines.size === 0 || isSubmitting} > - {t("administer_medicines")} ({selectedMedicines.size}) + {isSubmitting ? t("submitting") : `${t("administer_medicines")} (${selectedMedicines.size})`} </Button>src/types/emr/medicationRequest.ts (1)
95-96: Track technical debt: Remove code property after backend update.The TODO comment indicates this is a temporary change. Create a follow-up ticket to remove this property once the backend administration dosage is updated from
QuantitytoDosageQuantity.Would you like me to create a tracking issue for removing this temporary code once the backend is updated?
src/components/Medicine/MedicationRequestTable/index.tsx (1)
115-219: Excellent UI/UX improvements with comprehensive features.The UI implementation includes:
- Clear tab separation between prescriptions and administration
- Responsive search with instant feedback
- Proper handling of loading and empty states
- Horizontal scrolling for wide content
Consider adding keyboard shortcuts for common actions like searching and clearing the search input.
<input type="text" placeholder="Search medications..." value={searchQuery} onChange={(e) => setSearchQuery(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Escape') { + setSearchQuery(''); + } + }} className="flex-1 bg-transparent text-sm outline-none placeholder:text-muted-foreground" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
public/locale/en.json(11 hunks)src/Utils/request/api.tsx(2 hunks)src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx(2 hunks)src/components/Medicine/MedicationAdministration/AdministrationTab.tsx(1 hunks)src/components/Medicine/MedicationAdministration/MedicineAdminDialog.tsx(1 hunks)src/components/Medicine/MedicationAdministration/MedicineAdminForm.tsx(1 hunks)src/components/Medicine/MedicationAdministration/MedicineAdminSheet.tsx(1 hunks)src/components/Medicine/MedicationAdministration/utils.ts(1 hunks)src/components/Medicine/MedicationRequestTable/MedicineItem.tsx(1 hunks)src/components/Medicine/MedicationRequestTable/MedicineList.tsx(1 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(3 hunks)src/components/Medicine/MedicationsTable.tsx(1 hunks)src/components/Medicine/MultiValueSetSelect.tsx(1 hunks)src/components/Medicine/utils.ts(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx(3 hunks)src/components/Patient/PatientHome.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(3 hunks)src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx(3 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx(3 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(3 hunks)src/components/ui/button.tsx(1 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx(1 hunks)src/types/emr/medicationAdministration/medicationAdministration.ts(3 hunks)src/types/emr/medicationAdministration/medicationAdministrationApi.ts(1 hunks)src/types/emr/medicationRequest.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Cypress Tests
src/components/Patient/PatientDetailsTab/index.tsx
[error] 35-35: TypeScript error: Cannot find name 'Updates'
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: OSSAR-Scan
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (30)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (2)
49-49: LGTM! The encounterId prop is correctly typed.The addition of the required
encounterIdproperty aligns with the PR objective to filter symptoms by encounter.
285-285: LGTM! The encounterId prop is correctly destructured.The prop is properly added to the component's parameter list and matches the interface definition.
src/components/ui/button.tsx (1)
45-45: Verify the impact of changing the default variant.Changing the default variant from "default" to "primary" will affect all instances where the Button component is used without an explicit variant prop. This could lead to unexpected visual changes across the application.
Let's verify the Button component usage in the codebase:
✅ Verification successful
Change appears safe due to explicit variant usage pattern
The codebase demonstrates a consistent pattern where button variants are explicitly specified rather than relying on default values. This significantly reduces the risk of unintended visual changes from modifying the default variant.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all instances of Button component usage without explicit variant prop # Search for Button component imports echo "Button component imports:" rg "import.*Button.*from.*components/ui/button" # Search for Button usage without variant prop # Note: This might have false positives but helps identify potential impacts echo -e "\nButton usage without variant prop:" ast-grep --pattern '<Button $$$>' # Search for Button usage with explicit variants echo -e "\nButton usage with explicit variants:" ast-grep --pattern '<Button variant="$_" $$$>'Length of output: 65794
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (2)
118-136: LGTM! Implementation aligns with PR objectives.The conditional rendering of medication components based on
encounterIdeffectively addresses the bug where structured responses weren't being filtered by Encounter. The implementation is clean and handles the absence ofencounterIdgracefully.
Line range hint
138-174: Verify if appointment type should check for encounterId.While the implementation for allergy, symptom, diagnosis, and encounter types is consistent and well-structured, the appointment type doesn't follow the same pattern of checking for
encounterId. This might be intentional, but it's worth verifying.Let's verify if other appointment-related components in the codebase require
encounterId:src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (1)
48-48: LGTM! Props interface updated correctly.The
encounterIdproperty is properly typed as string in the interface.src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (1)
63-63: LGTM! Implementation matches the pattern.The changes correctly integrate the encounter context into the medication statement queries. The implementation is consistent with other components.
Also applies to: 94-94, 114-114
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (1)
63-63: Verify allergy data filtering behavior.The implementation looks correct, but since allergies are critical for patient safety, we should verify that filtering by encounter doesn't hide important allergy information from other encounters.
Also applies to: 120-120, 131-131
src/components/Medicine/MedicationAdministration/AdministrationTab.tsx (2)
152-164: Validate correct handling of older administration records
The logic accumulates the most recent occurrence date per medication. Confirm that older records aren’t mistakenly excluded from other computations if they need to be displayed or tracked.
313-314: Verify status transitions for discontinuation
When discontinuing a medication, it transitions to"ended". Verify that additional business rules (e.g., reason for discontinuation) are captured if required.src/types/emr/medicationAdministration/medicationAdministrationApi.ts (1)
9-20: Confirm return types of upsert endpoint
TheupsertMedicationAdministrationdefinition indicates the response type as an array ofMedicationAdministrationRequestobjects. Verify it matches the backend's actual response (often newly updated or created resources return read objects or references).src/components/Medicine/MedicationRequestTable/MedicineList.tsx (2)
9-14: LGTM! Well-structured props interface.The
MedicineListPropsinterface is well-defined with clear types and proper optional property marking.
16-40: LGTM! Clean and efficient implementation.The component follows React best practices:
- Proper key usage in map function
- Controlled component pattern for checkboxes
- Clear separation of concerns between selection and display
src/components/Medicine/MedicationAdministration/utils.ts (1)
18-19: Consider timezone handling in date formatting.Both start and end times are set to the same moment using
new Date(). This might need to account for timezone differences and duration.Would you like me to provide an implementation that handles timezones properly?
src/components/Patient/PatientDetailsTab/index.tsx (1)
70-70:⚠️ Potential issueFix TypeScript error: Updates component reference.
The pipeline is failing because the
Updatescomponent is still referenced inpatientTabsbut has been removed.Apply this diff to fix the error:
{ route: "updates", - component: Updates, + component: QuestionnaireResponsesList, },Likely invalid or redundant comment.
src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (1)
49-52: LGTM! Proper prop passing to QuestionnaireResponsesList.The addition of the
patientIdprop aligns with the PR objectives and maintains consistency with other components in the file.src/types/emr/medicationAdministration/medicationAdministration.ts (1)
46-46: LGTM! Type consistency improvement.The change from
QuantitytoDosageQuantityaligns the type with medication request definitions, ensuring consistent typing across the codebase.src/components/Medicine/MedicationsTable.tsx (1)
Line range hint
22-31: LGTM! Good function extraction.Making
getFrequencyDisplayexportable improves code reusability while maintaining its functionality.src/components/Medicine/MultiValueSetSelect.tsx (1)
58-58: LGTM! Proper propagation of disabled state.The addition of the
disabledprop to the Button component correctly propagates the disabled state from parent to child, ensuring consistent UI behavior.src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx (2)
17-18: LGTM! Props interface updated correctly.The changes to make
encounteroptional and add requiredpatientIdimprove component reusability while maintaining type safety.
128-141: LGTM! API parameters updated appropriately.The changes to use
patientIddirectly and make encounter conditional align with the interface changes and PR objectives.src/components/Medicine/MedicationAdministration/MedicineAdminForm.tsx (1)
45-48: Consider adding validation for lastAdministeredDate.The component accepts
lastAdministeredDateas an optional string but doesn't validate its format before using it withnew Date(). This could lead to runtime errors with invalid dates.src/components/Patient/PatientHome.tsx (1)
139-139: LGTM! Consistent prop passing.The addition of the
patientIdprop enables child components to access the patient identifier, which is necessary for the medication administration functionality.src/Utils/request/api.tsx (1)
657-663: LGTM! Well-structured API endpoint.The new medication administration endpoint follows the established API patterns:
- RESTful path structure
- Consistent with other patient-related endpoints
- Proper typing with
PaginatedResponsesrc/components/Medicine/MedicationRequestTable/index.tsx (2)
21-50: Well-structured EmptyState component with good UX.The EmptyState component is well-designed with:
- Clear prop interface
- Conditional messaging based on search state
- Consistent styling with the UI theme
65-101: Clean implementation of medication status management.Good implementation of medication status handling:
- Separate queries for active and stopped medications
- Efficient state management with
showStopped- Clear separation of concerns in data fetching
public/locale/en.json (4)
752-752: LGTM! Comprehensive status keys for medication administration workflow.The added status keys provide clear and concise states for tracking medication administration:
- "draft": For in-progress administration records
- "ended": For completed administration cycles
- "in_progress": For active administration
- "not_done": For skipped or cancelled administration
- "prescribed": For medications that are prescribed but not yet administered
- "taken": For successfully administered medications
Also applies to: 902-902, 1071-1071, 1386-1386, 1585-1585, 1985-1985
759-759: LGTM! Clear action and search labels.The added keys provide clear labels for medication-related actions:
- "edit_administration": For editing medication administration records
- "search_medicine": For medicine search functionality
Also applies to: 1810-1810
1137-1137: LGTM! Appropriate messages for various states.The added keys provide clear messages for:
- Past time administration confirmation
- Empty states for diagnoses and symptoms
Also applies to: 1322-1322, 1373-1373
Line range hint
752-1985: Verify consistency in terminology.Ensure consistent usage of terms across the application:
- "medicine" vs "medication"
- "administration" vs "administered"
✅ Verification successful
Terminology usage is consistent and follows healthcare standards
The codebase demonstrates intentional and consistent terminology usage:
- Backend/API layer uses formal "medication" terminology aligned with healthcare standards (FHIR)
- Frontend/UI layer uses more user-friendly "medicine" terminology for better user experience
- "Administration" related terms are used consistently across all layers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for inconsistent terminology usage echo "Checking for terminology consistency..." rg -i "medic(ine|ation)" rg -i "administ(ration|ered|er)"Length of output: 71229
| if (administrations?.results?.length) { | ||
| const sortedAdmins = [...administrations.results].sort( | ||
| (a, b) => | ||
| new Date(b.occurrence_period_start).getTime() - | ||
| new Date(a.occurrence_period_start).getTime(), | ||
| ); | ||
|
|
||
| if (!lastModifiedDate) { | ||
| setLastModifiedDate(new Date(sortedAdmins[0].occurrence_period_start)); | ||
| } | ||
| } | ||
| }, [administrations]); |
There was a problem hiding this comment.
Potential stale lastModifiedDate issue
Since lastModifiedDate is only set when no previous date exists, any new administration record after the component mounts won’t update this state (line 210). If tracking the latest modification time is important, consider removing the if (!lastModifiedDate) condition.
- if (!lastModifiedDate) {
+ // Always re-check if the new admin is newer
setLastModifiedDate(new Date(sortedAdmins[0].occurrence_period_start));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (administrations?.results?.length) { | |
| const sortedAdmins = [...administrations.results].sort( | |
| (a, b) => | |
| new Date(b.occurrence_period_start).getTime() - | |
| new Date(a.occurrence_period_start).getTime(), | |
| ); | |
| if (!lastModifiedDate) { | |
| setLastModifiedDate(new Date(sortedAdmins[0].occurrence_period_start)); | |
| } | |
| } | |
| }, [administrations]); | |
| if (administrations?.results?.length) { | |
| const sortedAdmins = [...administrations.results].sort( | |
| (a, b) => | |
| new Date(b.occurrence_period_start).getTime() - | |
| new Date(a.occurrence_period_start).getTime(), | |
| ); | |
| // Always re-check if the new admin is newer | |
| setLastModifiedDate(new Date(sortedAdmins[0].occurrence_period_start)); | |
| } | |
| }, [administrations]); |
| path: "/api/v1/patient/{patientId}/medication/administration/upsert/", | ||
| method: HttpMethod.POST, | ||
| TRes: Type<MedicationAdministrationRequest[]>, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Support partial failures
Consider whether the upsert endpoint should handle partial errors in case some medication administration requests fail, while others succeed. Returning a more detailed response with success/failure statuses may improve transparency.
| if (dose_range) { | ||
| return `${dose_range.low.value}${dose_range.low.unit.display} - ${dose_range.high.value}${dose_range.high.unit.display}`; | ||
| } else if (dose_quantity) { | ||
| return `${dose_quantity.value} ${dose_quantity.unit.display}`; | ||
| } | ||
|
|
||
| const { dose_quantity } = instruction.dose_and_rate; | ||
| if (!dose_quantity?.value || !dose_quantity.unit) return ""; | ||
|
|
||
| return `${dose_quantity.value} ${dose_quantity.unit.display}`; | ||
| return ""; |
There was a problem hiding this comment.
Handle missing or invalid dosage data
The logic gracefully checks for dose_range or dose_quantity. However, if dose_range.low.value or .high.value is absent or invalid, this may cause runtime errors. A more robust guard or fallback could help prevent potential crashes.
if (dose_range) {
+ if (!dose_range.low?.value || !dose_range.low?.unit || !dose_range.high?.value || !dose_range.high?.unit) {
+ return "";
+ }
return `${dose_range.low.value}${dose_range.low.unit.display} - ${dose_range.high.value}${dose_range.high.unit.display}`;
} else if (dose_quantity) {
+ if (!dose_quantity.value || !dose_quantity.unit) {
+ return "";
+ }
return `${dose_quantity.value} ${dose_quantity.unit.display}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (dose_range) { | |
| return `${dose_range.low.value}${dose_range.low.unit.display} - ${dose_range.high.value}${dose_range.high.unit.display}`; | |
| } else if (dose_quantity) { | |
| return `${dose_quantity.value} ${dose_quantity.unit.display}`; | |
| } | |
| const { dose_quantity } = instruction.dose_and_rate; | |
| if (!dose_quantity?.value || !dose_quantity.unit) return ""; | |
| return `${dose_quantity.value} ${dose_quantity.unit.display}`; | |
| return ""; | |
| if (dose_range) { | |
| if (!dose_range.low?.value || !dose_range.low?.unit || !dose_range.high?.value || !dose_range.high?.unit) { | |
| return ""; | |
| } | |
| return `${dose_range.low.value}${dose_range.low.unit.display} - ${dose_range.high.value}${dose_range.high.unit.display}`; | |
| } else if (dose_quantity) { | |
| if (!dose_quantity.value || !dose_quantity.unit) { | |
| return ""; | |
| } | |
| return `${dose_quantity.value} ${dose_quantity.unit.display}`; | |
| } | |
| return ""; |
| site: medication.dosage_instruction[0]?.site, | ||
| route: medication.dosage_instruction[0]?.route, | ||
| method: medication.dosage_instruction[0]?.method, |
There was a problem hiding this comment.
Add bounds checking for dosage_instruction array access.
The code directly accesses dosage_instruction[0] without checking if the array is empty, which could lead to runtime errors.
Apply this diff to add safety checks:
dosage: {
- site: medication.dosage_instruction[0]?.site,
- route: medication.dosage_instruction[0]?.route,
- method: medication.dosage_instruction[0]?.method,
+ site: medication.dosage_instruction?.[0]?.site,
+ route: medication.dosage_instruction?.[0]?.route,
+ method: medication.dosage_instruction?.[0]?.method,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| site: medication.dosage_instruction[0]?.site, | |
| route: medication.dosage_instruction[0]?.route, | |
| method: medication.dosage_instruction[0]?.method, | |
| site: medication.dosage_instruction?.[0]?.site, | |
| route: medication.dosage_instruction?.[0]?.route, | |
| method: medication.dosage_instruction?.[0]?.method, |
| {formatDosage(medication.dosage_instruction[0])} | ||
| </p> |
There was a problem hiding this comment.
Add null check for dosage_instruction array access.
Direct access to medication.dosage_instruction[0] could throw if the array is empty. Consider using optional chaining:
- {formatDosage(medication.dosage_instruction[0])}
+ {formatDosage(medication.dosage_instruction?.[0])}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {formatDosage(medication.dosage_instruction[0])} | |
| </p> | |
| {formatDosage(medication.dosage_instruction?.[0])} | |
| </p> |
| }); | ||
| }; | ||
|
|
||
| console.log("patientId", patientId); |
There was a problem hiding this comment.
Remove debug console.log statement.
Remove the debug console.log statement before merging to production.
- console.log("patientId", patientId);| <div className="space-y-2"> | ||
| <Label>{t("status")}</Label> | ||
| <Select | ||
| value={administrationRequest.status} | ||
| onValueChange={(value: MedicationAdministrationStatus) => | ||
| onChange({ ...administrationRequest, status: value }) | ||
| } | ||
| disabled={ | ||
| !!administrationRequest.id && | ||
| administrationRequest.status !== "in_progress" | ||
| } | ||
| > | ||
| <SelectTrigger> | ||
| <SelectValue placeholder={t("select_status")} /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {MEDICATION_ADMINISTRATION_STATUS.map((status) => ( | ||
| <SelectItem key={status} value={status}> | ||
| {t(status)} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding status transition validation.
The status selection logic should validate whether the transition from one status to another is valid according to the medication administration workflow.
<Select
value={administrationRequest.status}
onValueChange={(value: MedicationAdministrationStatus) =>
- onChange({ ...administrationRequest, status: value })
+ onChange({
+ ...administrationRequest,
+ status: validateStatusTransition(administrationRequest.status, value)
+ ? value
+ : administrationRequest.status,
+ })
}
disabled={
!!administrationRequest.id &&
administrationRequest.status !== "in_progress"
}
>Committable suggestion skipped: line range outside the PR's diff.
| const now = new Date().toISOString().slice(0, 16); | ||
| onChange({ | ||
| ...administrationRequest, | ||
| occurrence_period_start: now, | ||
| occurrence_period_end: now, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Potential timezone issue with date handling.
Using toISOString().slice(0, 16) might lead to timezone-related issues. Consider using a more robust date formatting approach.
-const now = new Date().toISOString().slice(0, 16);
+const now = new Date().toISOString();Committable suggestion skipped: line range outside the PR's diff.
| const { mutate: upsertAdministrations } = useMutation({ | ||
| mutationFn: mutate( | ||
| medicationAdministrationApi.upsertMedicationAdministration, | ||
| { | ||
| pathParams: { patientId: patientId }, | ||
| }, | ||
| ), | ||
| onSuccess: () => { | ||
| onOpenChange(false); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for mutation failures.
The mutation success case is handled, but there's no error handling. Consider adding error handling and user feedback.
const { mutate: upsertAdministrations } = useMutation({
mutationFn: mutate(
medicationAdministrationApi.upsertMedicationAdministration,
{
pathParams: { patientId: patientId },
},
),
onSuccess: () => {
onOpenChange(false);
},
+ onError: (error) => {
+ // Handle error and show user feedback
+ console.error('Failed to update administrations:', error);
+ // Consider adding a toast notification here
+ },
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { mutate: upsertAdministrations } = useMutation({ | |
| mutationFn: mutate( | |
| medicationAdministrationApi.upsertMedicationAdministration, | |
| { | |
| pathParams: { patientId: patientId }, | |
| }, | |
| ), | |
| onSuccess: () => { | |
| onOpenChange(false); | |
| }, | |
| }); | |
| const { mutate: upsertAdministrations } = useMutation({ | |
| mutationFn: mutate( | |
| medicationAdministrationApi.upsertMedicationAdministration, | |
| { | |
| pathParams: { patientId: patientId }, | |
| }, | |
| ), | |
| onSuccess: () => { | |
| onOpenChange(false); | |
| }, | |
| onError: (error) => { | |
| // Handle error and show user feedback | |
| console.error('Failed to update administrations:', error); | |
| // Consider adding a toast notification here | |
| }, | |
| }); |
| const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => { | ||
| e.preventDefault(); | ||
|
|
||
| const administrations = Array.from(selectedMedicines).map((id) => { | ||
| return administrationRequests[id]; | ||
| }); | ||
|
|
||
| upsertAdministrations({ | ||
| datapoints: administrations, | ||
| }); | ||
|
|
||
| onOpenChange(false); | ||
| setSelectedMedicines(new Set()); | ||
| setAdministrationRequests({}); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation before submission.
The submit handler should validate all administration requests before submission to ensure data integrity.
const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
const administrations = Array.from(selectedMedicines).map((id) => {
return administrationRequests[id];
});
+ // Validate all administrations before submission
+ const invalidAdministrations = administrations.filter(
+ (admin) => !isValidAdministrationRequest(admin)
+ );
+ if (invalidAdministrations.length > 0) {
+ // Handle invalid administrations
+ return;
+ }
upsertAdministrations({
datapoints: administrations,
});Committable suggestion skipped: line range outside the PR's diff.
Deploying care-fe with
|
| Latest commit: |
a1773bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://57f68a93.care-fe.pages.dev |
| Branch Preview URL: | https://medicine-administration-ui.care-fe.pages.dev |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
medicine-administration-ui
|
| Run status |
|
| Run duration | 05m 29s |
| Commit |
|
| Committer | Amjith Titus |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
9
|
| View all changes introduced in this branch ↗︎ | |
|
@nihal467 Needs testing |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/Medicine/MedicationAdministration/MedicineAdminDialog.tsx (1)
48-58:⚠️ Potential issueAdd error handling to mutation.
The mutation lacks error handling. Consider adding
onErrorcallback and showing a user-friendly error message.const { mutate: upsertAdministration, isPending } = useMutation({ mutationFn: mutate( medicationAdministrationApi.upsertMedicationAdministration, { pathParams: { patientId: patientId }, }, ), onSuccess: () => { onOpenChange(false); }, + onError: (error) => { + // Show error toast/notification + console.error('Failed to save medication administration:', error); + }, });
🧹 Nitpick comments (3)
src/components/Medicine/MedicationAdministration/MedicineAdminForm.tsx (2)
45-48: Evaluate initialization of "isPastTime" logic.When
administrationRequest.occurrence_period_start !== administrationRequest.occurrence_period_endoradministrationRequest.idis truthy, the form defaults to a "past time" scenario. This logic may conflate normal usage scenarios in which the user only wants to start and end at different times in the future. Consider making this condition more explicit if the goal is specifically to handle backdated administration events.
149-179: Ensure robust logic for scheduling past administrations.While toggling "isPastTime," the code sets both start and end times to the current timestamp if "no" is selected. If a user previously entered manual times, these are overwritten. Consider prompting the user to confirm or preserving the prior input if it was unintentional.
src/components/Medicine/MedicationAdministration/MedicineAdminDialog.tsx (1)
68-68: Consider adding max-height for better mobile UX.The dialog content might benefit from a max-height constraint with overflow scrolling for better mobile experience.
- <DialogContent className="sm:max-w-[525px]"> + <DialogContent className="sm:max-w-[525px] max-h-[90vh] overflow-y-auto">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Medicine/MedicationAdministration/MedicineAdminDialog.tsx(1 hunks)src/components/Medicine/MedicationAdministration/MedicineAdminForm.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
- GitHub Check: OSSAR-Scan
🔇 Additional comments (6)
src/components/Medicine/MedicationAdministration/MedicineAdminForm.tsx (4)
59-76: Confirm time zone handling withformatDistanceToNow.The
formatDistanceToNowfunction uses system locale settings. In multi-timezone environments, users may see inaccurate intervals if the times are stored or interpreted in UTC. Double-check that this display aligns with your workflow and that you account for any potential mismatch between local time and stored times.
113-136: Revisit status transition validation.This component changes the status with no guard to ensure valid transitions (e.g., "in_progress" → "completed"). Consider restricting disallowed transitions to adhere to medication administration workflow rules, or else you risk inconsistent states.
181-220: Validate "end_time" is not earlier than "start_time".When the user updates the end time, there is no check preventing an earlier end from preceding the previously set start. This could cause logical inconsistencies in administration periods.
Would you like me to provide a script to test for potential invalid administration periods throughout the code?
37-223: Overall structure and readability are good.The component is well-organized and leverages form elements effectively. Use of i18n, date-fns, and typed properties for medication administration offers clarity and maintainability.
src/components/Medicine/MedicationAdministration/MedicineAdminDialog.tsx (2)
1-30: LGTM! Well-structured imports and type definitions.The imports are well-organized, and the Props interface is properly typed with all necessary fields.
40-46: Consider handling state updates during pending mutations.There's a potential race condition where the state could be updated via
useEffectwhile a mutation is in progress. Consider disabling the effect or preserving the pending state whenisPendingis true.React.useEffect(() => { if (!isPending) { setAdministrationRequest(initialRequest); } }, [initialRequest, isPending]);
| const handleSubmit = () => { | ||
| upsertAdministration({ | ||
| datapoints: [administrationRequest], // Send as single-item array for upsert | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add form validation before submission.
Consider adding validation checks before submitting the form to ensure all required fields are filled and data is in the correct format.
const handleSubmit = () => {
+ // Validate required fields
+ if (!administrationRequest.dosage || !administrationRequest.administeredAt) {
+ // Show validation error
+ return;
+ }
upsertAdministration({
datapoints: [administrationRequest],
});
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSubmit = () => { | |
| upsertAdministration({ | |
| datapoints: [administrationRequest], // Send as single-item array for upsert | |
| }); | |
| }; | |
| const handleSubmit = () => { | |
| // Validate required fields | |
| if (!administrationRequest.dosage || !administrationRequest.administeredAt) { | |
| // Show validation error | |
| return; | |
| } | |
| upsertAdministration({ | |
| datapoints: [administrationRequest], // Send as single-item array for upsert | |
| }); | |
| }; |
|
👋 Hi, @amjithtitus09, |
…medicine-administration-ui
|
Things changed
|
|
Enhancement
non related issue
|
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Localization
New Features
Improvements
Bug Fixes